Skip to content

Conversation

@lzchen
Copy link
Member

@lzchen lzchen commented Nov 1, 2023

@github-actions github-actions bot added the Monitor - Exporter Monitor OpenTelemetry Exporter label Nov 1, 2023
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@lzchen lzchen merged commit 97e3d41 into Azure:main Nov 1, 2023
@lzchen lzchen deleted the docs branch November 1, 2023 19:32
@nhtgl
Copy link
Contributor

nhtgl commented Nov 23, 2023

We used this feature. Our whole metrics is based on namespaces. Can we get more info behind this change? The link in the description is private. Our client was really upset. We get that exporter is in beta, but this is kind of a breaking change.

@lzchen
Copy link
Member Author

lzchen commented Nov 27, 2023

@macieyng

Apologies about the poor experience with your client. We do monthly releases of the exporter and the distro packages. You can find the latest release notes for the exporter here. You can check this page in the future for any breaking changes. We mistakenly placed the namespace change under "bug fixes" and not "breaking changes" this time. As for this specific feature, by default, metric namespace should not be mapped from instrumentation name. However, we are currently working on an opt-in method to bring this feature back if needed, which will be available next release. I will be making a quick release (sometime by the end of this week) due to the urgency for your client. Thanks for your understanding.

@lzchen
Copy link
Member Author

lzchen commented Dec 4, 2023

@macieyng

Just a followup on the above. May I ask if depending on the namespace is absolutely necessary? What is the reason you need to use namespace (and not let say, metric name). Are you in need of identifying which instrumentation generated that specific metric (like opentelemetry.instrumentation.requests for example)?

@nhtgl
Copy link
Contributor

nhtgl commented Dec 5, 2023

I guess it's not necessary. For us namespace value was a service name. We implemented in this style some amount of our metrics and we have Azure Dashboards depending on it, that business is constantly looking at. So when we did update, prod dashboard went blank for one service. On our end we would have to make changes to dashboards and reimplement metric names. Like no we have namespace: order_service and metric name orders_count and we would probably have to change that to order_service.orders_count. Not a big deal, but it's just how we used it.

We would appreciate a solution that we not require as to do a lot of development, if that's possible. Currently we froze monitor exporter at 1.0.0b17.

@lzchen
Copy link
Member Author

lzchen commented Dec 6, 2023

@macieyng

By design, namespace was never supposed to be used so it was an erroroneous feature on our part. Metric groupings should be handled by metric attributes ("apply splitting" feature in metrics explorer). If you don't see an attribute that you can use that you can use to logically split the metrics the way you desire, we can expose that for you, but namespaces should not be used anymore.

@nhtgl
Copy link
Contributor

nhtgl commented Dec 7, 2023

Okay. We'll better focus on development on our end then.

@lzchen - as always - thank you for your support!

@wxpjimmy
Copy link

wxpjimmy commented Jan 10, 2024

@lzchen If namespace was never supposed to be used, should we remove it from the metrics dashboard? currently when you create a dashboard, you had to choose a namespace first. That's why customer might think they still need this.

If we decide not to bring it back, seems we also need to update the doc which still mentions that the metrics namespace can be customized.
image

@lzchen
Copy link
Member Author

lzchen commented Jan 10, 2024

@wxpjimmy

Can you paste a link to the doc that says this.

@lzchen
Copy link
Member Author

lzchen commented Jan 11, 2024

@wxpjimmy

Thanks for bringing this to our attention. We will work on correcting the docs.

@lzchen
Copy link
Member Author

lzchen commented Feb 27, 2024

@macieyng @wxpjimmy

We have decided to expose this feature via an opt-in. Here is the pr. This will be available in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Monitor - Exporter Monitor OpenTelemetry Exporter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants